-
Notifications
You must be signed in to change notification settings - Fork 5.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
BREAKING: disallow static import of local modules from remote modules #5050
BREAKING: disallow static import of local modules from remote modules #5050
Conversation
This commit changes module loading logic to disallow statically import local module (file:// scheme) from remote modules (http://, https:// schemes).
Ref #3401 |
This would break some nice use cases. One that comes to mind is filesystem based web server routes. |
How so? Dynamic imports are still working. |
Oh whoops, I misread the title - I missed the static part. Then this seems like a really nice change :-D |
@@ -0,0 +1,2 @@ | |||
[WILDCARD] | |||
Remote module are not allowed to statically import local modules. Use dynamic import instead. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In case of .js
-only imports the error is very sparse and has no stack trace...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better than nothing
// TODO(bartlomieju): duplicated from `state.rs::ModuleLoader::load` - deduplicate | ||
// Verify that remote file doesn't try to statically import local file. | ||
if let Some(referrer) = ref_specifier_.as_ref() { | ||
let referrer_url = referrer.as_url(); | ||
match referrer_url.scheme() { | ||
"http" | "https" => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This cruft will be removed during TS compiler refactor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…denoland#5050) This commit changes module loading logic to disallow statically import local module (file:// scheme) from remote modules (http://, https:// schemes).
@bartlomieju I've run into a nice (IMO) use-case that this logic prevents: I'm using an import map to remap one of my transitive dependencies to a local file, but when trying to bundle my app it fails with the error:
Can I work it around somehow? PS I submitted the issue: #7723, it's probably better to keep the discussion there |
@vovaguguiev currently there is no way. Would you mind opening an issue with your use case? |
This commit changes module loading logic to disallow statically import
local module (file:// scheme) from remote modules (http://, https://
schemes).